Remove -Wno-unused-function -Wno-tautological-compare -Wno-unused-value for clrjit projects#128271
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
Re-enables -Wunused-function for the JIT (by removing the repo-wide -Wno-unused-function for this directory on Unix/WASI) and fixes the warnings that surfaced. This includes guarding several helpers with the same #ifdef conditions as their callers, deleting a truly unused overload, and incidentally repairing two bugs (a missing assignment and a discarded expression) that the warning exposed.
Changes:
- Enable
-Wunused-functionfor clrjit targets via a new block insrc/coreclr/jit/CMakeLists.txt. - Guard target/DEBUG-only helpers (
RelopEvaluationResultString,TryGetStoreCoalescingConstantBits,MatchIntConstSelectValues/IntConstSelectOper,isLowSimdReg) with matching#ifdefs, and delete the unusedCorInfoType-takingisSupportedBaseTypeoverload. - Fix two latent bugs uncovered by the warning: a missing
id =assignment inGetHWIntrinsicIdForCmpOp(GT_GT/isScalar/xarch path) and discardedimpPopStack().valexpressions inimpIntrinsic.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/CMakeLists.txt | Strip -Wno-unused-function from the JIT directory's compile options on Unix/WASI. |
| src/coreclr/jit/scev.cpp | Wrap debug-only RelopEvaluationResultString in #ifdef DEBUG. |
| src/coreclr/jit/lower.cpp | Guard TryGetStoreCoalescingConstantBits for TARGET_XARCH/TARGET_ARM64. |
| src/coreclr/jit/ifconversion.cpp | Guard IntConstSelectOper/MatchIntConstSelectValues for TARGET_RISCV64. |
| src/coreclr/jit/emitxarch.cpp | Wrap debug-only isLowSimdReg in #ifdef DEBUG. |
| src/coreclr/jit/hwintrinsic.cpp | Remove unused CorInfoType-overload of isSupportedBaseType; remaining var_types overload is the one used at all call sites. |
| src/coreclr/jit/gentree.cpp | Fix missing id = assignment in scalar GT_GT case in GetHWIntrinsicIdForCmpOp. |
| src/coreclr/jit/importercalls.cpp | Replace impPopStack().val; no-op statements with impPopStack();. |
|
PTAL @dotnet/jit-contrib minor cleanup, I'll do the clean up for unused vars separately as it emits too many changes (and probably will not remove that flag for variables) |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The Attributes.BoxedValues test now passes after dotnet#127596 added custom attribute blob parsing and rewriting, but the entry was not removed from ILTrimExpectedFailures.txt. CLR_Tools_Tests fails with: Test 'Attributes.BoxedValues' is in the expected failures list but now passes. Remove it from ILTrimExpectedFailures.txt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
ping @dotnet/jit-contrib simple clean up, no diffs the idea is to enforce some warnings to prevent bugs like the one I fixed |
jakobbotsch
left a comment
There was a problem hiding this comment.
Seems fine to me given the small number of ifdefs, but I wouldn't want to enable warnings that require introducing a large number of ifdefs in the style of the ones introduced here.
Right, that's why i didn't remove |
|
/ba-g timeouts |
We currently define the followings for the entire native code:
my initial attempt to remove them all led to a very big diff so I decided to focus on JIT first.